Conversation
sleeepyjack
left a comment
There was a problem hiding this comment.
This PR satisfies my inner Monk. Very good!
| rapids_cuda_init_architectures(CUCO) | ||
|
|
||
| # this is needed for clang-tidy runs | ||
| set(CMAKE_EXPORT_COMPILE_COMMANDS ON) |
There was a problem hiding this comment.
This also enables us to use other development tools that use the clangd language server. Awesome!
| cuda_event_timer(benchmark::State &state, bool flush_l2_cache = false, cudaStream_t stream = 0) | ||
| cuda_event_timer(benchmark::State& state, | ||
| bool flush_l2_cache = false, | ||
| cudaStream_t stream = nullptr) |
There was a problem hiding this comment.
The default stream is commonly represented by (int)0 but I guess nullptr is fine, too.
There was a problem hiding this comment.
cudaStream_t is an alias of a pointer. clang-tidy will complain if we initialize it with 0.
| size_t num_pairs) | ||
| { | ||
| constexpr cudaStream_t stream = 0; | ||
| constexpr cudaStream_t stream{nullptr}; |
There was a problem hiding this comment.
I am curious about if we can use nullptr instead of 0.
https://docs.nvidia.com/cuda/cuda-c-programming-guide/index.html#default-stream
Kernel launches and host <-> device memory copies that do not specify any stream parameter, or equivalently that set the stream parameter to zero, are issued to the default stream. They are therefore executed in order.
There was a problem hiding this comment.
Right, 0 is the common way to initialize CUDA stream while according to the CUDA user guide, cudaStream_t is a type alias of CUstream_st * (see here) and clang-tidy will complain if it's initialized with 0. @karthikeyann @sleeepyjack For your reference, rmm is doing the same in rapidsai/rmm#857.
|
What's the state of this PR? This could be used to automatically catch bugs such as #510. |
I vaguely recall that some upstream libraries couldn't be built with clang. Let me prioritize this work. |
TBD